Skip to content

use overloaded web3 instance instead of single provider. required fo…#76

Merged
srt0422 merged 2 commits into
devfrom
bugfix-cycle-public-nodes
Sep 28, 2023
Merged

use overloaded web3 instance instead of single provider. required fo…#76
srt0422 merged 2 commits into
devfrom
bugfix-cycle-public-nodes

Conversation

@srt0422
Copy link
Copy Markdown
Contributor

@srt0422 srt0422 commented Sep 28, 2023

…r cycling through multiple public nodes

Comment thread src/plugins/eth/index.js
Comment thread src/plugins/contracts/index.js
Comment thread src/plugins/eth/web3.js
Comment thread src/plugins/eth/index.js
Comment on lines +32 to 34
web3,
web3Provider: web3.currentProvider,
web3SubscriptionProvider: web3SubscribAble.currentProvider,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is directly exposing the web3 and web3Provider objects. This could potentially lead to security issues as it allows direct access to the web3 provider and could be misused. Instead of directly exposing these objects, consider providing a set of methods that abstract the operations that can be performed on these objects. This way, you can control what operations are allowed and how they are performed, which can help to prevent misuse and improve security.


const web3 = new Web3(eth.web3Provider)
const web3 = eth.web3
const web3Subscriptionable = new Web3(plugins.eth.web3SubscriptionProvider)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is creating a new instance of Web3 using plugins.eth.web3SubscriptionProvider. However, there is no error handling in case the creation of the new Web3 instance fails. This could lead to unexpected behavior or crashes if the web3SubscriptionProvider is not properly configured or if there are network issues.

To improve this, you should wrap the creation of the new Web3 instance in a try-catch block and handle any potential errors appropriately. This could involve logging the error, retrying the operation, or failing gracefully with a user-friendly error message.

@srt0422 srt0422 merged commit 23cc390 into dev Sep 28, 2023
Comment thread src/plugins/eth/web3.js
Comment on lines 111 to 128
})
} else {
try {
result = originalFunctions[key].apply(this, args)
if (result !== undefined) {
if (new.target) {
function F(args) {
return originalFunction.apply(this, args)
}

F.prototype = originalFunction.prototype

return new F(args)
} else {
result = originalFunctions[key].apply(this, args)
}

if (typeof result !== "undefined") {
break
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handling in this code is not sufficient. In the catch block, the error is caught but nothing is done with it. This can make debugging difficult because you lose information about what caused the error.

To improve this, you should at least log the error. Depending on your application's requirements, you might also want to handle specific types of errors differently. For example, you might want to retry on certain types of errors, or report them to an error tracking service.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant